Conversation
|
Thanks for the contribution! Please review the labels and make any necessary changes. |
Summary of ChangesHello @chunjaemin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 애플리케이션 전반에 걸쳐 일관된 사용자 경험을 제공하기 위한 공용 드롭다운 컴포넌트를 도입합니다. 이를 통해 UI 개발의 효율성을 높이고, 디자인 시스템의 통일성을 강화하며, 향후 유사한 컴포넌트 개발 시 재사용성을 확보할 수 있습니다. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 공용 드롭다운 컴포넌트를 추가하고 관련 스타일 토큰을 정의합니다. 전반적으로 컴포넌트 구현이 잘 되어 있으며, 외부 라이브러리를 래핑하여 공통 인터페이스를 제공하는 좋은 접근 방식입니다. 몇 가지 유지보수성 및 일관성 개선을 위한 제안 사항이 있습니다. tokens.ts 파일의 색상 값들이 global.styles.css와 수동으로 동기화되어야 하는 점은 장기적으로 불일치를 유발할 수 있습니다. 또한 Select.tsx 컴포넌트 내에서 불필요한 useMemo 사용과 스타일링 방식의 불일치가 발견되었습니다. 자세한 내용은 각 파일에 대한 리뷰 코멘트를 참고해주세요.
| /** | ||
| * RN style 객체에서 사용하기 위한 토큰 상수. | ||
| * | ||
| * NOTE: | ||
| * - React Native style은 `var(--token)`을 직접 해석하지 못하므로, | ||
| * `global.styles.css`의 토큰 값을 여기에도 "동일한 값"으로 유지합니다. | ||
| * - 값 변경 시 `global.styles.css`와 함께 수정하세요. | ||
| */ | ||
|
|
||
| export const colorTokens = { | ||
| /** global.styles.css: --color-primary (blue-500) */ | ||
| primary: "#0068FE", | ||
| /** global.styles.css: --color-primary-tint (blue-100) */ | ||
| primaryTint: "#E5F6FE", | ||
|
|
||
| /** global.styles.css: --color-neutral (gray-100) */ | ||
| neutral: "#F4F4F5", | ||
| /** global.styles.css: --color-neutral-variant (gray-300) */ | ||
| neutralVariant: "#DBDDE1", | ||
|
|
||
| /** global.styles.css: --color-canvas (white) */ | ||
| canvas: "#FEFFFE", | ||
|
|
||
| /** global.styles.css: --color-danger (red-500) */ | ||
| danger: "#FF6562", | ||
|
|
||
| /** global.styles.css: --color-content-primary (black) */ | ||
| contentPrimary: "#040404", | ||
| /** global.styles.css: --color-content-secondary (gray-500) */ | ||
| contentSecondary: "#8E9398", | ||
| /** global.styles.css: --color-content-inverse (blue-50) */ | ||
| contentInverse: "#F4F6FE", | ||
| } as const; |
There was a problem hiding this comment.
tokens.ts와 global.styles.css에 색상 토큰이 분리되어 있어 두 파일 간의 값이 달라질 위험이 있습니다. 이는 장기적인 유지보수성을 저해할 수 있습니다. 예를 들어, 한쪽 파일의 값만 수정하면 UI 불일치가 발생할 수 있습니다.
이 문제를 해결하기 위해, 디자인 토큰을 JSON이나 YAML 같은 단일 소스 파일에서 관리하고, 빌드 스크립트를 통해 tokens.ts와 CSS 변수를 모두 생성하는 방식을 고려해보시는 것을 추천합니다. 이렇게 하면 토큰 값의 일관성을 보장할 수 있습니다.
| const dropdownData = useMemo(() => { | ||
| // react-native-element-dropdown은 disabled key가 없어서 | ||
| // item 렌더링/선택 로직에서 직접 처리한다. | ||
| return items; | ||
| }, [items]); |
There was a problem hiding this comment.
useMemo 훅이 여기서는 단순히 items prop을 반환하고 있어 메모이제이션의 이점이 없습니다. 코드를 단순화하기 위해 useMemo를 제거하고 Dropdown 컴포넌트의 data prop에 items를 직접 전달하는 것이 좋습니다. useMemo 내의 주석은 유용하지만, disabled 로직이 실제로 처리되는 onChange 핸들러 근처로 옮기는 것이 더 적절해 보입니다.
There was a problem hiding this comment.
저희 items가 대부분 고정 데이터일 거 같아서 useMemo 리뷰는 수용해도 될 거 같네요! 😁
| <Text | ||
| style={{ | ||
| fontFamily: "Pretendard-Regular", | ||
| fontSize: sizeToken.fontSize, | ||
| color: isSelected | ||
| ? colorTokens.primary | ||
| : colorTokens.contentPrimary, | ||
| }} | ||
| > | ||
| {item.label} | ||
| </Text> |
There was a problem hiding this comment.
컴포넌트 내 다른 부분에서는 className을 사용하여 스타일을 적용하고 있지만, renderItem의 Text 컴포넌트에서는 인라인 style 객체와 하드코딩된 fontFamily를 사용하여 스타일링 방식에 일관성이 부족합니다. placeholderStyle과 selectedTextStyle에도 동일한 문제가 있습니다.
유지보수성과 일관성을 높이기 위해 className을 사용하고, 폰트 이름 하드코딩을 피하는 것을 권장합니다. size prop에 따라 동적으로 클래스를 적용할 수 있습니다.
| <Text | |
| style={{ | |
| fontFamily: "Pretendard-Regular", | |
| fontSize: sizeToken.fontSize, | |
| color: isSelected | |
| ? colorTokens.primary | |
| : colorTokens.contentPrimary, | |
| }} | |
| > | |
| {item.label} | |
| </Text> | |
| <Text | |
| className={`font-regular ${ | |
| size === "sm" ? "text-sm" : "text-base" | |
| } ${ | |
| isSelected | |
| ? "text-primary" | |
| : "text-content-primary" | |
| }`} | |
| > | |
| {item.label} | |
| </Text> |
ahcgnoej
left a comment
ahcgnoej
left a comment
There was a problem hiding this comment.
고생하셨습니당 토큰파일 만들어주신 거 잘 썼습니다👍👍
| const dropdownData = useMemo(() => { | ||
| // react-native-element-dropdown은 disabled key가 없어서 | ||
| // item 렌더링/선택 로직에서 직접 처리한다. | ||
| return items; | ||
| }, [items]); |
There was a problem hiding this comment.
저희 items가 대부분 고정 데이터일 거 같아서 useMemo 리뷰는 수용해도 될 거 같네요! 😁
3 participants
📝 요약
⚙️ 작업 내용
| 외부 라이브러리는 nativeWind 연동이 안되서 tokens파일을 따로 만들었습니다.
완료된 컴포넌트 사진
아래 사진은 공용 드롭다운 컴포넌트로 로그인 과정에 쓰이는 드롭다운을 만들어본 예시 사진입니다.
🔗 관련 이슈
💬 리뷰어에게